Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (41)
📝 WalkthroughWalkthroughThe change adds route-specific selector dimensions to protocol metadata, schemas, CLI parsing, help output, browser targeting tests, and documentation. Unsupported ChangesRoute-aware target contracts
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant RouteRegistry
participant ArgvParser
participant BrowserExtension
CLI->>RouteRegistry: resolve route and selector dimensions
RouteRegistry->>ArgvParser: provide supported --window/--tab options
ArgvParser->>BrowserExtension: send validated target request
BrowserExtension-->>CLI: return targeted result and metadata
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abfaa2ef79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c50378a24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/protocol/src/target.ts (1)
17-21: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrefer
z.discriminatedUnionfor better performance and clearer error messages.Because
kindacts as a discriminator among the shapes, usingz.discriminatedUnion("kind", [...])provides O(1) evaluation and prevents misleading validation error messages compared to a standardz.union.♻️ Proposed refactor
-export const targetDimensionSelectorSchema = z.union([ +export const targetDimensionSelectorSchema = z.discriminatedUnion("kind", [ z.object({ kind: z.literal("active") }).strict(), z.object({ kind: z.literal("id"), id: z.number().int().nonnegative() }).strict(), z.object({ kind: z.literal("index"), index: z.number().int().nonnegative() }).strict(), ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/protocol/src/target.ts` around lines 17 - 21, Update targetDimensionSelectorSchema to use z.discriminatedUnion with "kind" as the discriminator and retain the existing active, id, and index object schemas unchanged.packages/cli/src/route-registry.ts (1)
154-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the generation of the selector usage suffix.
The array spread and ternary logic can be flattened into simple branching, making it slightly more direct and easier to read.
💡 Proposed refactor
-function withTargetSelectorUsage(help: string, selectorDimensions: CliRouteMetadata["selectorDimensions"]): string { - const selectorUsage = [ - ...(selectorDimensions === "window" || selectorDimensions === "both" ? ["[--window <target>]"] : []), - ...(selectorDimensions === "tab" || selectorDimensions === "both" ? ["[--tab <target>]"] : []), - ]; - return selectorUsage.length === 0 ? help : `${help} ${selectorUsage.join(" ")}`; -} +function withTargetSelectorUsage(help: string, selectorDimensions: CliRouteMetadata["selectorDimensions"]): string { + if (selectorDimensions === "neither") { + return help; + } + const usage = selectorDimensions === "both" + ? "[--window <target>] [--tab <target>]" + : `[--${selectorDimensions} <target>]`; + return `${help} ${usage}`; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/route-registry.ts` around lines 154 - 161, Update withTargetSelectorUsage to build the selector usage suffix through straightforward branching instead of conditional array spreads and ternaries. Preserve the existing ordering and inclusion rules for window, tab, both, and no selector dimensions, as well as the current help-string formatting.packages/cli/src/cli-tabs-targets.test.ts (1)
346-352: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the flag support condition.
The boolean logic covers all valid paths but can be simplified into direct checks, taking advantage of the
flagnaming structure directly matching the dimension values.💡 Proposed refactor
-function supportsSelectorFlag(selectorDimensions: "neither" | "window" | "tab" | "both", flag: "--tab" | "--window"): boolean { - return ( - (flag === "--tab" && (selectorDimensions === "tab" || selectorDimensions === "both")) || - (flag === "--window" && (selectorDimensions === "window" || selectorDimensions === "both")) - ); -} +function supportsSelectorFlag(selectorDimensions: "neither" | "window" | "tab" | "both", flag: "--tab" | "--window"): boolean { + if (selectorDimensions === "both") return true; + if (selectorDimensions === "neither") return false; + return flag === `--${selectorDimensions}`; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/cli-tabs-targets.test.ts` around lines 346 - 352, Update supportsSelectorFlag to simplify the boolean condition by deriving the dimension from the flag name and directly checking it against selectorDimensions, while preserving "both" support and the existing behavior for all valid flag and selector values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/cli/src/cli-tabs-targets.test.ts`:
- Around line 346-352: Update supportsSelectorFlag to simplify the boolean
condition by deriving the dimension from the flag name and directly checking it
against selectorDimensions, while preserving "both" support and the existing
behavior for all valid flag and selector values.
In `@packages/cli/src/route-registry.ts`:
- Around line 154-161: Update withTargetSelectorUsage to build the selector
usage suffix through straightforward branching instead of conditional array
spreads and ternaries. Preserve the existing ordering and inclusion rules for
window, tab, both, and no selector dimensions, as well as the current
help-string formatting.
In `@packages/protocol/src/target.ts`:
- Around line 17-21: Update targetDimensionSelectorSchema to use
z.discriminatedUnion with "kind" as the discriminator and retain the existing
active, id, and index object schemas unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2281a2db-9e4b-4bd9-84af-2ab8269beef7
📒 Files selected for processing (26)
docs/commands.mddocs/firefox-cli-spec.mdpackages/cli/src/argv-contracts.tspackages/cli/src/cli-tabs-targets.test.tspackages/cli/src/cli-target-contract.test.tspackages/cli/src/help.tspackages/cli/src/route-registry.test.tspackages/cli/src/route-registry.tspackages/cli/src/runner.tspackages/extension/src/browser-commands-targets.test.tspackages/protocol/src/browser/output.tspackages/protocol/src/metadata.tspackages/protocol/src/protocol-metadata-behavior.test.tspackages/protocol/src/protocol-request.test.tspackages/protocol/src/protocol-test-support.tspackages/protocol/src/registry/actions.tspackages/protocol/src/registry/browsing.tspackages/protocol/src/registry/content.tspackages/protocol/src/registry/core.tspackages/protocol/src/registry/define.tspackages/protocol/src/registry/index.tspackages/protocol/src/registry/pairing.tspackages/protocol/src/registry/phase8.tspackages/protocol/src/registry/registry.test.tspackages/protocol/src/target.tsskills/firefox-cli/SKILL.md
💤 Files with no reviewable changes (1)
- packages/protocol/src/browser/output.ts
Nek-12
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d766b1534b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
active, index, and ID selectors remain intentional.tab.new,window.select, andwindow.closesteps.tab select/window selecthelp and text output that selection brings Firefox forward to the user and does not establish durable CLI target state.Verification
bun run checkgit diff --check